-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Allow TriggerAuthentication defaults via ENV variables for HashiCorp Vault #5196
Conversation
Thank you for your contribution! 🙏 We will review your PR as soon as possible. While you are waiting, make sure to:
Learn more about: |
Signed-off-by: Bojan Zelic <[email protected]>
cee6115
to
9c024a3
Compare
Signed-off-by: Bojan Zelic <[email protected]>
Signed-off-by: Bojan Zelic <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this feature as it helps the vault integration (majority of the use cases have a single vault).
I'm not sure about the naming of the envs (I know that they are the hashicorp envs but KEDA supports more than it), maybe adding something like HASHICORP_*
prefix we can be more clear (or even more, DEFAULT_HASHICORP_*).
WDYT @tomkerkhove @zroubalik ?
Could you open PR's to helm chart and docs explaining this feature?
I do understand the problem, though I would actually prefer if we solve this in general for all config - thus having some global config for KEDA. If we start adding this kind of config via ENV variables the list would growth a lot in the future. |
I tend to agree on this |
@zroubalik @tomkerkhove I understand that the original design might lead to a some complexity long-term. What are your thoughts on any of the following alternatives for setting user-specified default values? Option 1) Mutating Webhook A user could specify a example:
and set defaults whenever the TriggerAuthentication CR is created to the above values. The defaults will be persisted on the custom resource itself. Option 2) Dynamic ENV variables example:
If you have any other suggestions/approaches please let me know; I'd really like to get this feature implemented |
I think we should discuss this on the issue and take #5229 (from @majusmisiak) in to mind as they both have similar goal - Make it easier to pull configuration from outside the scaledjob/triggerauth. In this case, envFrom or so would be better. Created #5233 to consolidate proposals |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
This issue has been automatically closed due to inactivity. |
This PR sets vault defaults if specified via ENV variables. When configuring the operator, you can set ENV variables so users don't have to redefine vault settings every time they use vault authentication;
example:
Checklist
Fixes # #4965
Relates to #